serviceability: authorize only new multicast roles - #4302
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The authorization logic change is localized, is mirrored in the SDK preflight, and is covered by new program and SDK regression tests plus changelog entries.
Pull request overview
This PR changes UpdateMulticastGroupRoles so it checks multicast allowlists only when the instruction would grant a new publisher or subscriber role, instead of re-authorizing roles the user already holds. This aligns serviceability-program behavior with the EdgeSeat feed subscription model so existing subscriptions remain valid when adding publishing rights.
Changes:
- Update onchain allowlist enforcement to run only for newly granted multicast roles.
- Update the Rust SDK command preflight checks to mirror the onchain behavior.
- Add regression tests (program + SDK) and document behavior in the changelog.
File summaries
| File | Description |
|---|---|
| smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs | Skip SDK-side allowlist checks for roles the user already has; add a regression unit test. |
| smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs | Gate onchain allowlist checks on whether the role is newly granted for that group. |
| smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs | Add an integration test that preserves an existing subscriber role when adding publisher after sub allowlist removal. |
| CHANGELOG.md | Add Serviceability and SDK entries describing the behavior change. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
nikw9944
left a comment
nikw9944
left a comment
There was a problem hiding this comment.
Logic looks right: skipping the allowlist for an already-held role is state-neutral, and a genuinely new unallowlisted role is still rejected. One stale comment on the authorization model.
Docs & comments (non-blocking)
smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs:333— The comments describing the multicast authorization model now contradict the code beneath them. This loop comment says each group "must be on the pass's allowlist", and thecheck_mgroup_allowlistsdoc (line 63-68) says a caller runs the allowlist check or the EdgeSeat feed gate, "never neither".
Summary
Testing
This change addresses infra#2596.